Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add transitions to Sections bg and line #2491

Merged
merged 4 commits into from
Nov 26, 2024
Merged

Conversation

viktorrenkema
Copy link
Contributor

@viktorrenkema viktorrenkema commented Sep 25, 2024

This PR adds a line animation transitions for the current active chapter in the Sections/ToC, respecting prefers-reduced-motion.

Related Slack thread.

Before:

CleanShot.2024-09-25.at.18.00.27.mp4

After:

CleanShot.2024-11-26.at.17.49.11.mp4

Copy link

changeset-bot bot commented Sep 25, 2024

⚠️ No Changeset found

Latest commit: ff74ae8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

argos-ci bot commented Sep 25, 2024

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) 👍 Changes approved 37 changed Dec 31, 2024, 11:12 PM

@SamyPesse
Copy link
Member

@viktorrenkema can you run bun run changeset to write a changelog entry?

While I think it's a great addition, I'm slightly concerned on the animated background, I find the background to be quite heavy/distracting.

@zenoachtig what do you think? Here is a preview link: https://a859aaea.gitbook-open.pages.dev/docs.gitbook.com/content-editor/editor/navigation

@viktorrenkema
Copy link
Contributor Author

@viktorrenkema can you run bun run changeset to write a changelog entry?

While I think it's a great addition, I'm slightly concerned on the animated background, I find the background to be quite heavy/distracting.

@zenoachtig what do you think? Here is a preview link: https://a859aaea.gitbook-open.pages.dev/docs.gitbook.com/content-editor/editor/navigation

I see what you mean, I also considered using a light grey instead, that'd be a bit more subtle. But yeah I'll let @zenoachtig provide his take :)

CleanShot.2024-09-26.at.16.39.56.mp4

@zenoachtig
Copy link
Contributor

I agree with @SamyPesse's concern, i think the gray background is indeed a lot subtler and as a bonus it matches the left sidebar better 😄

The square corners on this element annoy me a bit, but I don't think rounding the two right corners is a good idea either. Probably nothing worth doing about.

@viktorrenkema
Copy link
Contributor Author

I agree with @SamyPesse's concern, i think the gray background is indeed a lot subtler and as a bonus it matches the left sidebar better 😄

The square corners on this element annoy me a bit, but I don't think rounding the two right corners is a good idea either. Probably nothing worth doing about.

Yeah the sharp corners bug me too, but it also matches the 'was this helpful' picker below etc. we could prolly pimp this component in general, it can be prettier. but not worthwhile for now, and this pr provides the backbone for the interactive part and we can always tweak the exact design of it if we were to redesign the whole component.

@SamyPesse wdyt, keep the suggested gray background, or remove the background part altogether?

@BrettJephson
Copy link
Contributor

Just having a look at the preview branch. I think this looks great and had a couple of minor suggestions:

  1. When you have hovered over an item and then move away the highlight returns suddenly to the current item. Compare to how Remote do it with a fade out and I think the latter looks a bit slicker.
  2. We could maybe do something with focus as well, I notice Remote moves the highlight to the focused element when you change focus even though you can still move the selection with cursor too.

@viktorrenkema viktorrenkema force-pushed the viktor/transition-sections branch from eab8feb to 1cf5bcf Compare November 26, 2024 16:47
@viktorrenkema
Copy link
Contributor Author

Just having a look at the preview branch. I think this looks great and had a couple of minor suggestions:

  1. When you have hovered over an item and then move away the highlight returns suddenly to the current item. Compare to how Remote do it with a fade out and I think the latter looks a bit slicker.
  2. We could maybe do something with focus as well, I notice Remote moves the highlight to the focused element when you change focus even though you can still move the selection with cursor too.

Great points yeah, I hadn't noticed the former. The background effect is being removed from this PR but we have some ideas on bringing it in somewhere else, so duly noted.

@viktorrenkema viktorrenkema force-pushed the viktor/transition-sections branch from 1cf5bcf to d11c7d5 Compare November 26, 2024 16:51
@viktorrenkema viktorrenkema force-pushed the viktor/transition-sections branch from d11c7d5 to 8253c34 Compare November 26, 2024 16:53
Copy link
Contributor

github-actions bot commented Nov 26, 2024

GitBook Preview
Latest commit: https://8380afc8.gitbook-open.pages.dev
PR: https://pr2491.gitbook-open.pages.dev

Copy link
Contributor

@BrettJephson BrettJephson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@viktorrenkema viktorrenkema force-pushed the viktor/transition-sections branch from 5d24ef6 to 2fc108b Compare November 26, 2024 18:34
@viktorrenkema viktorrenkema merged commit 568266b into main Nov 26, 2024
9 checks passed
@viktorrenkema viktorrenkema deleted the viktor/transition-sections branch November 26, 2024 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants